fix: make install textfile busy by zenixls2 · Pull Request #558 · sipeed/picoclaw

zenixls2 · 2026-02-21T00:21:26Z

📝 Description

make install from bot will get textfile busy message, since it's still running

🗣️ Type of Change

🐞 Bug fix (non-breaking change which fixes an issue)
✨ New feature (non-breaking change which adds functionality)
📖 Documentation update
⚡ Code refactoring (no functional changes, no api changes)

🤖 AI Code Generation

🤖 Fully AI-generated (100% AI, 0% Human)
🛠️ Mostly AI-generated (AI draft, Human verified/modified)
👨‍💻 Mostly Human-written (Human lead, AI assisted or none)

🔗 Related Issue

📚 Technical Context (Skip for Docs)

Reference URL:
Reasoning:

🧪 Test Environment

Hardware:
OS:
Model/Provider:
Channels:

📸 Evidence (Optional)

Click to view Logs/Screenshots

☑️ Checklist

My code/docs follow the style of this project.
I have performed a self-review of my own changes.
I have updated the documentation accordingly.

Copilot

Pull request overview

This PR fixes the "text file busy" error that occurs when running make install while the picoclaw binary is currently running. The solution implements an atomic file replacement pattern using a temporary file and mv, which is a standard approach for replacing executables that may be in use.

Changes:

Adds a temporary file suffix variable (INSTALL_TMP_SUFFIX=.new)
Modifies the install target to use atomic file replacement (copy to temp → chmod → atomic move)

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-21T01:59:30Z

pkg/tools/shell_test.go

+	if err := os.MkdirAll(workspace, 0o755); err != nil {
 		t.Fatalf("failed to create workspace: %v", err)
 	}
-	if err := os.MkdirAll(outsideDir, 0755); err != nil {
+	if err := os.MkdirAll(outsideDir, 0o755); err != nil {
 		t.Fatalf("failed to create outside dir: %v", err)
 	}

 	tool := NewExecTool(workspace, true)
-	result := tool.Execute(context.Background(), map[string]interface{}{
+	result := tool.Execute(context.Background(), map[string]any{
 		"command":     "pwd",
 		"working_dir": outsideDir,
 	})

 	if !result.IsError {
 		t.Fatalf("expected working_dir outside workspace to be blocked, got output: %s", result.ForLLM)
 	}
 	if !strings.Contains(result.ForLLM, "blocked") {
 		t.Errorf("expected 'blocked' in error, got: %s", result.ForLLM)
 	}
 }

 // TestShellTool_WorkingDir_SymlinkEscape verifies that a symlink inside the workspace
 // pointing outside cannot be used as working_dir to escape the sandbox.
 func TestShellTool_WorkingDir_SymlinkEscape(t *testing.T) {
 	root := t.TempDir()
 	workspace := filepath.Join(root, "workspace")
 	secretDir := filepath.Join(root, "secret")
-	if err := os.MkdirAll(workspace, 0755); err != nil {
+	if err := os.MkdirAll(workspace, 0o755); err != nil {
 		t.Fatalf("failed to create workspace: %v", err)
 	}
-	if err := os.MkdirAll(secretDir, 0755); err != nil {
+	if err := os.MkdirAll(secretDir, 0o755); err != nil {
 		t.Fatalf("failed to create secret dir: %v", err)
 	}
-	os.WriteFile(filepath.Join(secretDir, "secret.txt"), []byte("top secret"), 0644)
+	os.WriteFile(filepath.Join(secretDir, "secret.txt"), []byte("top secret"), 0o644)


The changes to shell_test.go (octal literal format and map type alias) appear to be unrelated to the PR's stated purpose of fixing "text file busy" errors during installation. These are code style improvements that should be mentioned in the PR description or submitted as a separate PR. The PR description claims this is a bug fix for the install target, but these test file changes are purely stylistic improvements that don't relate to the installation issue.

xiaket

LGTM

Makefile

Copilot

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-22T15:32:29Z

Makefile

 	@mkdir -p $(INSTALL_BIN_DIR)
-	@cp $(BUILD_DIR)/$(BINARY_NAME) $(INSTALL_BIN_DIR)/$(BINARY_NAME)
-	@chmod +x $(INSTALL_BIN_DIR)/$(BINARY_NAME)
+	# Copy binary with temporary suffix to ensure atomic update


The comment says "Copy binary with temporary suffix to ensure atomic update" but this isn't fully accurate. The atomic update is achieved by the mv -f command on line 106, not just by copying with a temporary suffix. Consider updating the comment to: "Copy binary with temporary suffix, then atomically replace to avoid 'text file busy' error"

Suggested change

# Copy binary with temporary suffix to ensure atomic update

# Copy binary with temporary suffix, then atomically replace to avoid 'text file busy' error

Copilot · 2026-02-22T15:32:30Z

pkg/tools/registry_test.go

+	params map[string]any
 	result *ToolResult
 }

-func (m *mockRegistryTool) Name() string                       { return m.name }
-func (m *mockRegistryTool) Description() string                { return m.desc }
-func (m *mockRegistryTool) Parameters() map[string]interface{} { return m.params }
-func (m *mockRegistryTool) Execute(_ context.Context, _ map[string]interface{}) *ToolResult {
+func (m *mockRegistryTool) Name() string               { return m.name }
+func (m *mockRegistryTool) Description() string        { return m.desc }
+func (m *mockRegistryTool) Parameters() map[string]any { return m.params }
+func (m *mockRegistryTool) Execute(_ context.Context, _ map[string]any) *ToolResult {


The changes to replace interface{} with any in this test file appear unrelated to the PR's stated purpose of fixing the "text file busy" error during installation. While this is a valid code modernization (Go 1.18+ type alias), it should ideally be in a separate PR or the PR description should mention this refactoring.

… to overwrite the file with non-atomic operation

xiaket

LGTM, thanks!

… to overwrite the file with non-atomic operation (sipeed#558)

Copilot AI review requested due to automatic review settings February 21, 2026 00:21

Copilot started reviewing on behalf of zenixls2 February 21, 2026 00:21 View session

Copilot AI reviewed Feb 21, 2026

View reviewed changes

zenixls2 force-pushed the fix/ota-textfile-busy branch from 18e7fdb to 255ebbb Compare February 21, 2026 01:34

Copilot AI review requested due to automatic review settings February 21, 2026 01:57

Copilot started reviewing on behalf of zenixls2 February 21, 2026 01:57 View session

Copilot AI reviewed Feb 21, 2026

View reviewed changes

zenixls2 force-pushed the fix/ota-textfile-busy branch from 539ca48 to 2893448 Compare February 21, 2026 17:21

xiaket approved these changes Feb 22, 2026

View reviewed changes

xiaket reviewed Feb 22, 2026

View reviewed changes

Makefile Show resolved Hide resolved

zenixls2 requested review from Copilot and xiaket February 22, 2026 06:22

zenixls2 force-pushed the fix/ota-textfile-busy branch from 2893448 to 053ebb2 Compare February 22, 2026 15:05

Copilot started reviewing on behalf of zenixls2 February 22, 2026 15:05 View session

Copilot AI reviewed Feb 22, 2026

View reviewed changes

zenixls2 force-pushed the fix/ota-textfile-busy branch 2 times, most recently from 260eae4 to c21042c Compare February 22, 2026 15:27

Copilot AI review requested due to automatic review settings February 22, 2026 15:29

Copilot started reviewing on behalf of zenixls2 February 22, 2026 15:29 View session

Copilot AI reviewed Feb 22, 2026

View reviewed changes

fix: make install should be aware of the textfile busy since it tries…

a2a3571

… to overwrite the file with non-atomic operation

zenixls2 force-pushed the fix/ota-textfile-busy branch from 8fe7ddd to a2a3571 Compare February 23, 2026 09:15

xiaket approved these changes Feb 23, 2026

View reviewed changes

xiaket merged commit 6d487a1 into sipeed:main Feb 23, 2026
2 checks passed

davidburhans pushed a commit to davidburhans/picoclaw that referenced this pull request Feb 23, 2026

fix: make install should be aware of the textfile busy since it tries…

5cc4ec1

… to overwrite the file with non-atomic operation (sipeed#558)

liangzhang-keepmoving pushed a commit to liangzhang-keepmoving/picoclaw that referenced this pull request Mar 2, 2026

fix: make install should be aware of the textfile busy since it tries…

aa00430

… to overwrite the file with non-atomic operation (sipeed#558)

hyperwd pushed a commit to hyperwd/picoclaw that referenced this pull request Mar 5, 2026

fix: make install should be aware of the textfile busy since it tries…

3a375f0

… to overwrite the file with non-atomic operation (sipeed#558)

renesul pushed a commit to renesul/OK that referenced this pull request Mar 14, 2026

fix: make install should be aware of the textfile busy since it tries…

44bcf4c

… to overwrite the file with non-atomic operation (sipeed#558)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: make install textfile busy#558

fix: make install textfile busy#558
xiaket merged 1 commit intosipeed:mainfrom
zenixls2:fix/ota-textfile-busy

zenixls2 commented Feb 21, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Feb 21, 2026

Uh oh!

xiaket left a comment

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Feb 22, 2026

Uh oh!

Copilot AI Feb 22, 2026

Uh oh!

xiaket left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

	# Copy binary with temporary suffix to ensure atomic update
	# Copy binary with temporary suffix, then atomically replace to avoid 'text file busy' error

Conversation

zenixls2 commented Feb 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

📝 Description

🗣️ Type of Change

🤖 AI Code Generation

🔗 Related Issue

📚 Technical Context (Skip for Docs)

🧪 Test Environment

📸 Evidence (Optional)

☑️ Checklist

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Feb 21, 2026

Choose a reason for hiding this comment

Uh oh!

xiaket left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Feb 22, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 22, 2026

Choose a reason for hiding this comment

Uh oh!

xiaket left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

zenixls2 commented Feb 21, 2026 •

edited

Loading